Skip to content

feat(commandrunner): log every command the library executes - #97

Open
Seitanas wants to merge 1 commit into
scality:mainfrom
Seitanas:feature/add_command_logging
Open

Seitanas wants to merge 1 commit into
scality:mainfrom
Seitanas:feature/add_command_logging

Conversation

@Seitanas

Copy link
Copy Markdown

Hello,
thank you for creating this project.
I found it very useful, but it would be nice to include command logging for debug purposes.

This is a PR to enable that.

Everything this library does on a host is a vendor CLI invocation behind a commandrunner.CommandRunner, so that interface is the one place where all executed commands can be observed. Logging goes there as a decorator rather than into each adapter: commandrunner.Logging wraps any runner, records the invocation on an slog.Logger, and returns the wrapped output and error unchanged. It is opt-in -- a consumer injects the decorated runner where it would have injected the concrete one -- and it covers the runners and adapters added later for free, since they all sit behind the same port.

One record per invocation carries the binary, the arguments, the duration and, on failure, the error. Successful commands are recorded at slog.LevelInfo, which WithLevel lowers, and failures at slog.LevelError. ErrNoLogicalDrives reports an empty inventory rather than a failure, so it stays at the success level instead of logging a controller with no volume as an error.

The output itself is never logged, only its size: vendor payloads carry drive serials and other identifying data. The arguments are the ones the adapter asked for, so a runner that appends flags of its own does so after the decorator has seen them and the storcli2/perccli2 JSON output flag is absent from the record; both are documented where a reader would look for them.

Each runner now reports the binary it invokes through CommandPath(), so a record names the tool that ran. The decorator forwards its wrapped runner's path, which also keeps it transparent to another decorator, and falls back to the runner's type for a runner without one, such as a test mock.

The legacy megaraid.Runner parses its own output instead of returning bytes, so it cannot share the decorator. megaraid.LoggingRunner decorates it the same way and emits through commandrunner.LogCommand, keeping one log shape across the library; it records no payload size, since that runner exposes none.

Everything this library does on a host is a vendor CLI invocation behind a
commandrunner.CommandRunner, so that interface is the one place where all
executed commands can be observed. Logging goes there as a decorator rather
than into each adapter: commandrunner.Logging wraps any runner, records the
invocation on an slog.Logger, and returns the wrapped output and error
unchanged. It is opt-in -- a consumer injects the decorated runner where it
would have injected the concrete one -- and it covers the runners and adapters
added later for free, since they all sit behind the same port.

One record per invocation carries the binary, the arguments, the duration and,
on failure, the error. Successful commands are recorded at slog.LevelInfo,
which WithLevel lowers, and failures at slog.LevelError. ErrNoLogicalDrives
reports an empty inventory rather than a failure, so it stays at the success
level instead of logging a controller with no volume as an error.

The output itself is never logged, only its size: vendor payloads carry drive
serials and other identifying data. The arguments are the ones the adapter
asked for, so a runner that appends flags of its own does so after the
decorator has seen them and the storcli2/perccli2 JSON output flag is absent
from the record; both are documented where a reader would look for them.

Each runner now reports the binary it invokes through CommandPath(), so a
record names the tool that ran. The decorator forwards its wrapped runner's
path, which also keeps it transparent to another decorator, and falls back to
the runner's type for a runner without one, such as a test mock.

The legacy megaraid.Runner parses its own output instead of returning bytes, so
it cannot share the decorator. megaraid.LoggingRunner decorates it the same way
and emits through commandrunner.LogCommand, keeping one log shape across the
library; it records no payload size, since that runner exposes none.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Seitanas
Seitanas requested a review from a team as a code owner September 22, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant